Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Cami Opal - A tool for evaluating taxonomic metagenome profilers #6096

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

Albert-Ber
Copy link
Contributor

Tool details for Cami Opal

  • Name: Cami Opal
  • Description: A tool to assess taxonomic metagenome profilers, predicting and evaluating the presence and relative abundance of microorganisms in microbial community DNA samples using metrics like Unifrac error and L1-norm error, and visual representations for these data.
  • Homepage: Cami Opal
  • License: License

Checklist for submission

FOR CONTRIBUTOR:

  • - I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • - License permits unrestricted use (educational + commercial)
  • - This PR adds a new tool or tool collection
  • - This PR updates an existing tool or tool collection
  • - This PR does something else (explain below)

I look forward to any feedback and thank you for your consideration.

Copy link
Contributor

@bernt-matthias bernt-matthias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments inline.

Out of interest: What is the difference or connection to https://github.com/galaxyproject/tools-iuc/tree/main/tools/cami_amber

tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/taxonkit/profile2cami/profile2cami.xml Outdated Show resolved Hide resolved
tools/taxonkit/profile2cami/.shed.yml Outdated Show resolved Hide resolved
@paulzierep
Copy link
Contributor

Some comments inline.

Out of interest: What is the difference or connection to https://github.com/galaxyproject/tools-iuc/tree/main/tools/cami_amber

amber is the cami binning evaluation tool (you need to provide a gold standard bin assignment / and optionally taxonomy for each bin) - so for DAS TOOL or other binners, opal is the cami evaluation tool for taxonomic composition - so Kraken2 / MetaPhlan ... in theory OPAL should also work for amplicon data (in terms of taxonomic assignment benchmarking) ... another thing to look into ....

@Albert-Ber Albert-Ber closed this Jun 24, 2024
@Albert-Ber Albert-Ber reopened this Jun 24, 2024
@Albert-Ber
Copy link
Contributor Author

I apologize for the confusion caused by the multiple commits in this pull request. I have addressed the issues and made the necessary adjustments to the code. Please take another look and provide any further feedback.

Thank you for your assistance!

Best regards

Copy link
Contributor

@paulzierep paulzierep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can u also fix @bernt-matthias comments ?

tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
Comment on lines 103 to 108
<param argument="--time" type="text" value="" optional="true"
label="Comma-separated runtimes in hours"
help="Comma-separated runtimes in hours" />
<param argument="--memory" type="text" value="" optional="true"
label="Comma-separated memory usages in gigabytes"
help="Comma-separated memory usages in gigabytes" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would allow adding it as optional, one could get this info from galaxy and add manually

tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@paulzierep paulzierep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor changes please

tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami-opal/cami_opal.xml Outdated Show resolved Hide resolved
@paulzierep
Copy link
Contributor

Can you add one test with multiple samples as input ?

Copy link
Contributor

@paulzierep paulzierep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes please

tools/cami_opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami_opal/cami_opal.xml Outdated Show resolved Hide resolved
tools/cami_opal/cami_opal.xml Show resolved Hide resolved
tools/cami_opal/.shed.yml Outdated Show resolved Hide resolved
tools/cami_opal/cami_opal.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@paulzierep paulzierep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes please, then good from my side

tools/cami_opal/macros.xml Show resolved Hide resolved
tools/cami_opal/cami_opal.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@paulzierep paulzierep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a catch description, then good to go

tools/cami_opal/cami_opal.xml Outdated Show resolved Hide resolved
## Copy the results to the specified output folder
&& mkdir '$htmlreport.extra_files_path'
&& cp output/results.html $htmlreport
&& cp -r output/* '$htmlreport.extra_files_path'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do all output files needs to be in this folder or only a handful that are part of the HTML?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most files should remain, as I haven't tested which ones are essential and which can be removed. So, I would recommend that you keep all of them in the folder for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How large is such a folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the detailed examples form: Here - they are approximately 20 MB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants